[RISCV] Set __GCC_CONSTRUCTIVE_SIZE/__GCC_DESTRUCTIVE_SIZE to 64 for riscv64 #162986
Conversation
…riscv64 These two macros were added in llvm#89446. But the values may not be reasonable for RV64 systems because most of them have a cache line size 64B.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-risc-v Author: Pengcheng Wang (wangpc-pp) ChangesThese two macros were added in #89446. But the values may not be reasonable for RV64 systems because most Full diff: https://github.com/llvm/llvm-project/pull/162986.diff 2 Files Affected:
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index d8b0e64c90dd6..215f82f65d8ef 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -125,10 +125,6 @@ class RISCVTargetInfo : public TargetInfo {
ParsedTargetAttr parseTargetAttr(StringRef Str) const override;
llvm::APInt getFMVPriority(ArrayRef<StringRef> Features) const override;
- std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
- return std::make_pair(32, 32);
- }
-
bool supportsCpuSupports() const override { return getTriple().isOSLinux(); }
bool supportsCpuIs() const override { return getTriple().isOSLinux(); }
bool supportsCpuInit() const override { return getTriple().isOSLinux(); }
@@ -178,6 +174,10 @@ class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
resetDataLayout("e-m:e-p:32:32-i64:64-n32-S128");
}
+ std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
+ return std::make_pair(32, 32);
+ }
+
bool setABI(const std::string &Name) override {
if (Name == "ilp32e") {
ABI = Name;
@@ -208,6 +208,10 @@ class LLVM_LIBRARY_VISIBILITY RISCV64TargetInfo : public RISCVTargetInfo {
resetDataLayout("e-m:e-p:64:64-i64:64-i128:128-n32:64-S128");
}
+ std::pair<unsigned, unsigned> hardwareInterferenceSizes() const override {
+ return std::make_pair(64, 64);
+ }
+
bool setABI(const std::string &Name) override {
if (Name == "lp64e") {
ABI = Name;
diff --git a/clang/test/Preprocessor/init-riscv.c b/clang/test/Preprocessor/init-riscv.c
new file mode 100644
index 0000000000000..36b2b5a06b8c6
--- /dev/null
+++ b/clang/test/Preprocessor/init-riscv.c
@@ -0,0 +1,12 @@
+// REQUIRES: riscv-registered-target
+
+// RUN: %clang_cc1 -E -dM -triple=riscv32 < /dev/null | \
+// RUN: FileCheck -match-full-lines -check-prefixes=RV32 %s
+// RUN: %clang_cc1 -E -dM -triple=riscv64 < /dev/null | \
+// RUN: FileCheck -match-full-lines -check-prefixes=RV64 %s
+
+// RV32: #define __GCC_CONSTRUCTIVE_SIZE 32
+// RV32: #define __GCC_DESTRUCTIVE_SIZE 32
+
+// RV64: #define __GCC_CONSTRUCTIVE_SIZE 64
+// RV64: #define __GCC_DESTRUCTIVE_SIZE 64
\ No newline at end of file
|
wangpc-pp
requested review from
asb,
jrtc27,
kito-cheng,
lukel97,
preames and
topperc
|
We found this issue accidentally. I don't know if we should document this in psabi or elsewhere, but I think the GCC should change it as well. cc @kito-cheng |
|
How do we deal with these in the ABI? We do not want people relying on these macros in their public interfaces, much like the |
I don't have a clear thought on this. Quoted from itanium-cxx-abi/cxx-abi#74:
We may expose the |
|
I wondered if we wanted a quick note in the C api doc instead of the ABI. I do agree with itanium that they should not be treated as stable, but i think it's good to explicitly note that somewhere, rather than exposing them without any relevant guidance. |
We have those docs: https://clang.llvm.org/docs/LanguageExtensions.html#gcc-destructive-size-and-gcc-constructive-size
|
|
GCC current settings:
|
|
I added some RISC-V specific documentation here: riscv-non-isa/riscv-c-api-doc#129 |
5 participants
These two macros were added in #89446.
But the values may not be reasonable for RV64 systems because most
of them have a cache line size 64B.